Skip to content

support sql transactions #10617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

adamgreenhall
Copy link
Contributor

this allows the con argument of pd.read_sql/pd.read_sql_query to be a sqlalchemy Session object.

this allows for the use of temporary tables.

@jorisvandenbossche jorisvandenbossche added the IO SQL to_sql, read_sql, read_sql_query label Jul 19, 2015
@jorisvandenbossche jorisvandenbossche added this to the 0.17.0 milestone Jul 19, 2015
@jorisvandenbossche
Copy link
Member

@adamgreenhall Thanks for the contribution!
Can you provide a bit more context? (I never used the ORM part of SQLAlchemy, only the Core part) But as a Session has a begin method and execute method, I suppose read_sql would just work regardless you provide it a Session or connection? (as the support for connections was just added in #10105, which should already allow temporary tables when using a Connection)

Why does it need sqlalchemy 1.0.0 ?

The reason your tests fail, is because it seems to run the tests with the fallback mode (although you put it in the correct test class). But this seems to indicate that it does not recognizes it as an sqlalchemy engine/connection/session, and therefore tries to use it as a DBAPI connection in fallback mode (hence the error 'has no attribute 'cursor'')

@adamgreenhall
Copy link
Contributor Author

@jorisvandenbossche - #10105 does the trick with sqlalchemy.engine.Connectable. I didn't check the docs on master before adding this - learned my lesson for next time. Thanks!

ps. Not sure what the sqlalchemy recommended way to handle transactions is now - I think sessions got added in 1.0 - that was just the first thing I found in the docs, but the with engine.begin(): code seems simpler.

@jorisvandenbossche
Copy link
Member

But

In [46]: isinstance(session, sqlalchemy.engine.Connectable)
Out[46]: False

so the current code is not going to work for a Session.
(I never used Sessions, but I can imagine that support for it would be useful for people using it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO SQL to_sql, read_sql, read_sql_query
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants